Skip to content

Conversation

devongovett
Copy link
Member

@devongovett devongovett commented Jul 21, 2022

Closes #3240. Closes #3087.

We were inconsistent about which types were exported from React Aria and React Stately packages. Mostly it was just if we needed it elsewhere, then it was exported. In addition, developers often needed to install @react-types packages in addition to the code packages to get the corresponding type definitions, leading to annoyance.

This ensures that all types that are the inputs or outputs of hooks or components (e.g. prop and state objects) are exported. In addition, it re-exports types that come from @react-types packages in the corresponding @react-stately, @react-aria, and @react-spectrum packages. Finally, it re-exports all public types from the corresponding mono packages. This means developers usually only need to install one package to get both the code and types they need.

Where they were not already exported (and therefore public API), and the naming was inconsistent, I also tried to address this before they become public. We will need to be more mindful of naming conventions for types going forward.

This was mostly automated using a script, but with manual review of all changes.

@adobe-bot
Copy link

snowystinger
snowystinger previously approved these changes Jul 21, 2022
Copy link
Member

@snowystinger snowystinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing jumps out at me as wrong, I see the backwards-compat one for the TimePicker/TimeField.
I'm assuming nothing changed in the @types because that's all already public, and what we want to export already is.

@adobe-bot
Copy link

Copy link
Member

@LFDanLu LFDanLu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, one question about the naming convention

Comment on lines +22 to +25
export type {GridProps, GridAria} from './useGrid';
export type {GridCellAria, GridCellProps} from './useGridCell';
export type {GridRowGroupAria} from './useGridRowGroup';
export type {GridRowProps, GridRowAria} from './useGridRow';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that the aria component hook options now follow "AriaCOMPONENTOptions" or "AriaCOMPONENTProps". At the risk of being overly pedantic, should the grid hook props follow this same convention?

Just trying to clarify the naming convention for the future

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes ideally, but, I couldn't change some of them that were already exported...

I think the convention is:

  • Aria___Props if it's props. Most of these come from @react-types
  • Aria___Options if we extended the props to add additional options only for the hook (e.g. additional refs)
  • ___Aria for the return types

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, GridProps were already exported, I'm fine keeping the other grid props naming in line with that then. Thanks for the clarification

@devongovett devongovett merged commit 7cebb4e into main Jul 26, 2022
@devongovett devongovett deleted the export-types branch July 26, 2022 23:24
Vinnl added a commit to mozilla/fx-private-relay that referenced this pull request Aug 31, 2022
The old OverlayProps was no longer available. This is presumably
the result of adobe/react-spectrum#3326.
Vinnl added a commit to mozilla/fx-private-relay that referenced this pull request Sep 14, 2022
The old OverlayProps was no longer available. This is presumably
the result of adobe/react-spectrum#3326.
Vinnl added a commit to mozilla/fx-private-relay that referenced this pull request Sep 14, 2022
The old OverlayProps was no longer available. This is presumably
the result of adobe/react-spectrum#3326.
Vinnl added a commit to mozilla/fx-private-relay that referenced this pull request Sep 14, 2022
The old OverlayProps was no longer available. This is presumably
the result of adobe/react-spectrum#3326.
Vinnl added a commit to mozilla/fx-private-relay that referenced this pull request Oct 5, 2022
The old OverlayProps was no longer available. This is presumably
the result of adobe/react-spectrum#3326.
Vinnl added a commit to mozilla/fx-private-relay that referenced this pull request Oct 5, 2022
The old OverlayProps was no longer available. This is presumably
the result of adobe/react-spectrum#3326.
Vinnl added a commit to mozilla/fx-private-relay that referenced this pull request Oct 17, 2022
The old OverlayProps was no longer available. This is presumably
the result of adobe/react-spectrum#3326.
Vinnl added a commit to mozilla/fx-private-relay that referenced this pull request Oct 17, 2022
The old OverlayProps was no longer available. This is presumably
the result of adobe/react-spectrum#3326.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

react-aria and react-stately should re-export all types Why are interfaces e.g. TableProps, part of the top-level external API, not exported?
5 participants